Conversation
There was a problem hiding this comment.
Pull request overview
Updates link-checking configuration and documentation, plus bumps spec test submodules.
Changes:
- Updated
spec_testssubmodule pointers forhed-testsandhed-schemas. - Adjusted Lychee configuration and GitHub Actions workflow to broaden link checking and add exclusions.
- Added a Funding section to the README.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec_tests/hed-tests | Updates submodule commit reference. |
| spec_tests/hed-schemas | Updates submodule commit reference. |
| pyproject.toml | Reformats Ruff exclude list string quoting. |
| lychee.toml | Expands Lychee exclusions (local URLs + spec_tests paths). |
| README.md | Adds project funding acknowledgment section. |
| .github/workflows/links.yaml | Runs Lychee against built docs HTML plus key markdown files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Review SummaryOne important issue, one suggestion. Important
The patterns Suggestion
The new Funding paragraph ends without a newline character. Minor POSIX issue that causes noisy diffs. Everything else looks good: extending the lychee workflow to also check |
Review: Updated lychee and READMEThis PR cleanly replaces Black with ruff format and updates the link checker configuration. The mechanical changes (removing Important
Suggestion README.md line 185 has a pre-existing typo: "To iew the built documentation" → "To view". Worth fixing since nearby lines are already changed in this PR. |
|
PR Review: Clean migration from black/codespell to ruff-format/typos with consistent updates across CI, docs, and config. One suggestion posted as inline comment on lychee.toml (missing spec_tests/hed-specification in exclude_path for consistency with pyproject.toml ruff config). Otherwise LGTM. |
|
PR Review placeholder - will be replaced |
|
PR Review: Updated lychee and README. Clean tooling migration: black+codespell to ruff format+typos, pytest to python -m unittest discover, and lychee expanded to check README / RELEASE_GUIDE / CONTRIBUTING. Documentation updates throughout are consistent and correct. IMPORTANT - tests/ excluded from typos spell-checking (pyproject.toml line 128): The previous codespell config skipped spec_tests/ but not tests/. Excluding the entire tests/ directory removes spell-checking from test-file comments, docstrings, and assertion messages - spelling errors in those strings (e.g. error messages verified in assertIn calls) would go undetected. If the goal is to avoid false positives from Python identifiers or HED-specific strings, consider adding those terms to [tool.typos.default.extend-words] instead of silencing the whole directory. SUGGESTION - Stale comment in lychee.toml (line 35): the comment above exclude_path reads 'These are Sphinx theme template files with Jinja2 variables that should not be checked' but the block now also contains spec_tests/ submodule paths that have nothing to do with Sphinx templates. Everything else looks correct: the hed-specification submodule removal is handled consistently across .gitmodules, .gitignore, ruff excludes, and lychee paths; the typos extend-words whitelist mirrors the old codespell ignore list; and the 127 to 120 line-length documentation fix matches the pre-existing ruff line-length = 120 setting. |
No description provided.